Skip to content

feat(thread_aware_core): add the stable core crate for thread-aware state - #643

Open
martintmk wants to merge 61 commits into
mainfrom
user/martintomka/20260806-stabilize-thread-aware
Open

feat(thread_aware_core): add the stable core crate for thread-aware state#643
martintmk wants to merge 61 commits into
mainfrom
user/martintomka/20260806-stabilize-thread-aware

Conversation

@martintmk

@martintmk martintmk commented Aug 6, 2026

Copy link
Copy Markdown
Member

Summary

Adds thread_aware_core, the stable vocabulary that thread-aware libraries share. Nothing
else in the workspace changes.

  • ThreadAware notifies a value that it has moved, via
    relocate(source: Option<&Thread>, destination: &Thread). The call is advisory: a value
    must remain correct if it is never called, called twice, or called with a Thread it has
    never seen. That single property is what lets the method be infallible and lets a runtime
    call it opportunistically.
  • Thread records where a value runs — Owner (which runtime owns it), a
    std::thread::ThreadId, and NumaNode (the memory closest to that thread). Fields are
    private and read through accessors.
  • Owner identity is assigned, not supplied: every new owner is unique, so two runtimes
    alive at once cannot collide. It also reports min_threads, the smallest number of threads
    its runtime runs, so a value can pre-size per-thread state before it has seen any of them.
  • NumaNode is an opaque u32 newtype built with an inherent const fn new(u32).
  • Nothing reaches a consumer's dependency graph; the only manifest entry is a test-only
    dev-dependency. The std feature is on by default and adds implementations for HashMap,
    Path and PathBuf. With default-features = false the crate needs only alloc and
    pointer-width atomics; Thread then loses its thread id and cannot be constructed, leaving
    Owner and NumaNode readable so a no_std library can still implement the trait.
  • docs/DESIGN.md records why the crate is split out, and the rules that let Thread gain
    coordinates later without breaking callers or changing behaviour.
    docs/STABILIZATION.md records the stable boundary.

Why Owner assigns its own identity

Callers used to choose the number, and the documentation warned at length that a collision
was "worse than a slowdown" and that keeping owners distinct was the embedding application's
problem. Taking the identity from a process-wide counter removes the hazard outright rather
than documenting it. Equality and hashing use identity alone.

min_threads is a floor, not an estimate. 0 means the runtime promises none and spawns on
demand, which falls through HashMap::with_capacity as a no-op, so both cases share one
expression at the call site. It was named threads_hint first; in std, _hint already means
a bound — size_hint returns a lower bound plus an optional upper, and overshooting is a bug
— so "hint" would have contradicted the suffix while leaving 0 a magic value.

Why the ids take new rather than From<u16>

A From<integer> impl on a permanently stable type is a one-shot commitment. Every way of
evolving one is breaking, verified against rustc:

Later change Result
Replace From<u16> with From<u32> breaks every caller passing a typed u16 (E0277)
Add From<u32> beside From<u16> breaks unsuffixed literals: from(1) falls back to i32 (E0277)
Add From<i32> no error at all — silently re-resolves from(1) to the new impl

cargo semver-checks 0.48 reports "no semver update required" for all three, so none of them
would be caught in review. NumaNode is therefore u32-backed with an inherent new. The
accepted width already exceeds any node count real hardware reaches — Windows' own topology
APIs report NUMA nodes as USHORT — and a wider or fallible constructor can still be added
later under a different name without touching existing callers.

Provided implementations

Containers forward to what they hold; primitives and other types with nothing tied to a
thread get an empty implementation. Three cases are deliberately absent:

  • References (&str, &Path) — relocating through one would adapt something the value
    only borrows, and whoever owns it is relocated on its own account.
  • Cow — relocating a borrowed one has to clone it into owned storage first, which is
    too much work to hide behind an advisory call. It can return later if a caller wants it.
  • Arc and sets — whether a shared allocation should split per thread depends on what it
    holds, and mutating a set element could change its hash and corrupt the container.

Naming

thread_aware_core::Thread shares a name with std::thread::Thread but is a different kind
of thing: an immutable coordinate that owns no OS resource. Importing both unqualified is
E0252, and both expose an identical id(&self) -> ThreadId. This is called out in the crate
docs and in a # Relation to std::thread::Thread section on the type. Raised deliberately
here in case reviewers would prefer a different name.

Scope

thread_aware is unchanged and still ships its own Affinity-based API. Nothing depends
on thread_aware_core yet; adoption is left for a later change. The diff against main is
the new crate plus its workspace wiring (Cargo.toml, Cargo.lock, CHANGELOG.md,
README.md) and nothing more.

Validation

  • 25 unit tests and 14 doctests with --all-features; 25 unit tests and 12 doctests with
    --no-default-features. One doctest is ignored: it demonstrates
    #[derive(ThreadAware)], which lives in thread_aware, and compiling it here would
    require the dependency the example exists to argue against.
  • Clippy in both feature configurations, rustdoc with -D warnings --cfg docsrs, formatting,
    spelling, and the generated README all pass.
  • Static assertions pin the auto traits (Send, Sync, Unpin, UnwindSafe,
    RefUnwindSafe) on all three public types, and assert_obj_safe! pins dyn-compatibility of
    ThreadAware. Both guard silent regressions: a coordinate added later that is not Sync
    would strip that property with no signature change, and a defaulted-but-generic trait method
    would keep every impl compiling while breaking every Box<dyn ThreadAware>.
  • Documentation and API shape went through repeated independent model-backed reviews. Those
    found real defects rather than wording nits: a counter width that would wrap and reissue a
    live identity, a claim that ThreadId is the operating-system thread id (it explicitly is
    not), an equality claim that was false under no_std, and an Eq/Hash test that passed
    vacuously because it compared a value with its own copy.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 3a761577-6422-4fd1-a0f7-88807d7f6134
@martintmk martintmk added the agency-rocket Touched by a rocket skill label Aug 6, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 3a761577-6422-4fd1-a0f7-88807d7f6134
@martintmk martintmk changed the title docs(thread_aware): add stabilization notes feat(stabilization)!: stabilize thread_aware 1.0 Aug 6, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 3a761577-6422-4fd1-a0f7-88807d7f6134
@martintmk martintmk changed the title feat(stabilization)!: stabilize thread_aware 1.0 feat(stabilization)!: thread_aware 1.0 Aug 6, 2026
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (66c4d12) to head (c375097).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #643   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files         584      586    +2     
  Lines       62913    62994   +81     
=======================================
+ Hits        62913    62994   +81     
Flag Coverage Δ
linux 100.0% <100.0%> (?)
linux-arm 100.0% <100.0%> (?)
windows 100.0% <100.0%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 392f99fd-c9b0-40c7-aad7-3e87a132b16e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 392f99fd-c9b0-40c7-aad7-3e87a132b16e
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

✅ Version increments look sufficient

cargo semver-checks compared the 1 crate(s) this PR publishes against their previous version-bump commit in git history. Every version increment is sufficient for the detected API changes.

Crate Baseline Baseline commit This PR Minimum required Status
thread_aware_core new crate 0.1.0 0.1.0 ✅ ok

This check is informational and does not block the merge.

View the check run

Comment thread crates/thread_aware_core/Cargo.toml
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 392f99fd-c9b0-40c7-aad7-3e87a132b16e
Make the stable core crate dependency- and feature-free, and remove external-type implementations and their benchmarks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 392f99fd-c9b0-40c7-aad7-3e87a132b16e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 392f99fd-c9b0-40c7-aad7-3e87a132b16e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 392f99fd-c9b0-40c7-aad7-3e87a132b16e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 392f99fd-c9b0-40c7-aad7-3e87a132b16e
Comment thread crates/thread_aware_core/src/impls.rs
Keep the core crate dependency-free while providing Path, PathBuf, and HashMap implementations behind an opt-in std feature.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 392f99fd-c9b0-40c7-aad7-3e87a132b16e
Comment thread crates/seatbelt/Cargo.toml Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 392f99fd-c9b0-40c7-aad7-3e87a132b16e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 392f99fd-c9b0-40c7-aad7-3e87a132b16e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 392f99fd-c9b0-40c7-aad7-3e87a132b16e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 392f99fd-c9b0-40c7-aad7-3e87a132b16e
Comment thread crates/thread_aware/src/__private.rs Outdated
Comment thread crates/thread_aware/src/affinity.rs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8f9c8e6e-73c6-47c0-830e-2f674656eceb
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8f9c8e6e-73c6-47c0-830e-2f674656eceb
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8f9c8e6e-73c6-47c0-830e-2f674656eceb
Comment thread crates/anyspawn/src/spawner.rs Outdated
Comment thread crates/tick/Cargo.toml Outdated
`cargo mutants` replaced `<impl Hash for Owner>::hash` with `()` and no
test noticed: the only hash assertion checked that two owners sharing an
identity hash *equally*, which a no-op hash satisfies trivially.

Assert the converse as well -- two distinct owners with the same thread
count must hash apart -- which fails under exactly that mutation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e215e3bc-7b6e-4743-afb4-55779bebd24b
Copilot AI review requested due to automatic review settings August 27, 2026 10:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

crates/thread_aware_core/src/thread.rs:89

  • Owner::new uses AtomicUsize::fetch_add, which wraps on overflow. That would violate the documented guarantee that every new Owner is unique by potentially reissuing an identity once the counter wraps (especially on 32-bit targets). Consider using fetch_update with checked_add to fail fast on exhaustion without mutating the counter into a wrapped state.
    pub fn new(min_threads: usize) -> Self {
        Self {
            id: NEXT_OWNER.fetch_add(1, Ordering::Relaxed),
            min_threads,
        }

crates/thread_aware_core/src/thread_aware.rs:165

  • This crate consistently marks #[cfg(test)] mod tests with #[cfg_attr(coverage_nightly, coverage(off))] to keep test-only lines from counting against the 100% coverage gate (e.g. crates/tick/src/error.rs). This test module is missing that attribute.
#[cfg(test)]
mod tests {

Comment thread crates/thread_aware_core/src/impls.rs
Resolved a conflict in the workspace Cargo.toml: main cascaded
thread_aware, thread_aware_macros and thread_aware_macros_impl to
0.11.0, while this branch adds thread_aware_core. Kept main's version
bumps and retained the new thread_aware_core = 0.1.0 entry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 698f459b-8a21-45d8-a46f-1b39e71df42a
Copilot AI review requested due to automatic review settings August 27, 2026 15:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (1)

crates/thread_aware_core/src/thread_aware.rs:165

  • Tests modules in this repo are consistently annotated with #[cfg_attr(coverage_nightly, coverage(off))] so unit-test-only lines don’t count against the coverage gate. This mod tests is missing that attribute (unlike thread.rs / impls.rs in the same crate).
#[cfg(test)]
mod tests {

Review flagged the single-element case of `impl_transfer_tuple!` as
broken. It is not: the arm emits a literal comma after the head, so the
1-tuple expands to `let (A,) = self` and `impl<A,> ThreadAware for (A,)`,
both of which are correct (a trailing comma is legal in a generic
parameter list).

The existing coverage could not show this, though. `test_tuples` uses
`(42,)`, and `i32::relocate` is a no-op, so the assertion holds whether
or not the element was ever visited. Add a test built on `Tracker`,
whose `relocate` flips an observable flag, covering the 1-tuple, the
2-tuple and the 12-element maximum.

Verified the test is load-bearing: reproducing the reported defect, by
dropping that literal comma, makes the 1-tuple bind the whole tuple and
call its own impl again, overflowing the stack.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 698f459b-8a21-45d8-a46f-1b39e71df42a
Copilot AI review requested due to automatic review settings August 27, 2026 15:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (1)

crates/thread_aware_core/src/thread_aware.rs:165

  • The unit-test module here is missing #[cfg_attr(coverage_nightly, coverage(off))]. Other test modules in this new crate already apply it (e.g., thread.rs and impls.rs), and omitting it can cause test-only lines to count against the coverage gate.
#[cfg(test)]
mod tests {

…cessors

Vec now relocates through `as_mut_slice`, matching the array impl instead of
repeating the slice loop. VecDeque keeps its own loop: its buffer is split, so
it has no single slice to delegate to.

Cell and RefCell were byte-identical, so they collapse into one macro arm. The
internal macros move off the old "transfer" vocabulary onto the public
`ThreadAware`/`relocate` names, and the stale comment above the leaf macro is
replaced with one that says what the macro emits.

Adds `#[inline]` to the trivial cross-crate accessors on `Owner`, `NumaNode`
and `Thread`, plus the `()` impl. These are non-generic, exported and sit on
the relocate path, which is rule 1 in docs/performance.md. `Owner::new` keeps
its atomic RMW uninlined as requested. `Owner::eq` is included as well: it is
the id comparison relocation actually performs, so inlining its callers but not
it would be half a change. `Hash for Owner` is generic and already an inlining
candidate, so it falls under rule 2 and is left alone.

Renames the four `test_`-prefixed tests to behaviour-style names, and renames
the `NumaNode::new` parameter to `node`.

The `no_std` `Eq`/`Hash` shape cannot be pinned by a unit test: `id` is gated on
`any(test, feature = "std")`, so `cfg(test)` restores it and a test gated on
`not(feature = "std")` still sees three fields. Measured directly --
`size_of::<Thread>()` is 32 under `--no-default-features --lib`. The new
`tests/no_std_surface.rs` links the crate as an ordinary dependency, where the
gate resolves on the feature alone, and pins the two-field shape there. The
`Thread` docs now state that equality and hashing compare owner and NUMA node
alone without `std`, and a comment at the derive records why the unit tests
cannot cover it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 698f459b-8a21-45d8-a46f-1b39e71df42a
Copilot AI review requested due to automatic review settings August 28, 2026 13:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (1)

crates/thread_aware_core/src/thread_aware.rs:165

  • The repo consistently marks #[cfg(test)] mod tests blocks with #[cfg_attr(coverage_nightly, coverage(off))] to keep non-executed test-only code from counting against the coverage gate (see e.g. crates/benchmarking/src/lib.rs:155-157). This tests module is missing that attribute, so it should be added for consistency with the workspace’s coverage setup.
#[cfg(test)]
mod tests {

…`Thread`

`Owner` names one live runtime, so making duplicates effortless worked against
what the type is for. It keeps `Clone`, and `Thread::owner` now returns
`&Owner` rather than handing out a copy on every call.

Call sites that reused an owner after moving it into `Thread::new` now clone
explicitly, which is the cost the change is meant to make visible. Comparisons
against the accessor take a reference, and the `hash_of` test helper borrows.

`owner_copy_preserves_the_thread_count` becomes
`owner_clone_preserves_the_thread_count` and now also asserts the clone keeps
the identity, not just the count. An `assert_not_impl_any!(Owner: Copy)` pins
the decision so it cannot be re-derived by accident.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 698f459b-8a21-45d8-a46f-1b39e71df42a
Copilot AI review requested due to automatic review settings August 28, 2026 13:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

crates/thread_aware_core/src/thread_aware.rs:165

  • The internal #[cfg(test)] mod tests block here is missing #[cfg_attr(coverage_nightly, coverage(off))], which this repo commonly uses to keep unit-test scaffolding from counting against the coverage gate (e.g., thread.rs and impls.rs in this crate, and tick/src/fmt/mod.rs). Adding the attribute keeps this file consistent with the established coverage setup.
#[cfg(test)]
mod tests {

Comment on lines +11 to +15
use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher};

use static_assertions::assert_impl_all;
use thread_aware_core::{NumaNode, Owner, Thread};
#[derive(Clone, Debug)]
pub struct Owner {
id: usize,
min_threads: usize,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how min_threads is supposed to be used?
considering that it doesn't participate in Hash and PartialEq implementations, it might be better to remove it.

alternative suggestion:

  • add OwnerId type that stores only id: usize
  • remove PartialEq, Hash impl blocks for owner

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

min_threads does not participate in Owner equality as it is only a hint to downstream customers for cases where they are reserving capacity for collections holding thread state.

Each instance of Owner gets it's own unique identity and we only have a single constructor. Later, if more information is needed on owner we can add Owner::builder() APIs where more properties can be set.

OwnerId can be added later to Owner::id if necessary, but currently it's not needed. At most, our code is comparing equality of 2 owners, which is already implemented correctly.

@Vaiz Evgenii (Vaiz) Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so what's the purpose of this field in Owner struct?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's to hold more information about owner that consuming side might need. At some point owner could gain more properties.

@Vaiz Evgenii (Vaiz) Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing I don't like the most here is that Owner combines two concepts with different semantics: identity and runtime metadata. That's what creates those awkward comparison impl blocks.

At the same time, if the goal of this PR is stabilization of thread_aware, and you already suggesting breaking changes, we should add Builder type right away, and may constructor methods private. Moreover, unless there is a defendable use case for min_threads, it's better to remove it from the type completely. Adding new fields when we have builder is not a breaking change, while removing it is.

@martintmk martintmk Aug 31, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing I don't like the most here is that Owner combines two concepts with different semantics: identity and runtime metadata. That's what creates those awkward comparison impl blocks.

But it indeed has both. You cannot create 2 owners with the same properties that are equal. Owner identity is generated, each new owner get's completely new identity so there is no point comparing individual properties such as min_threads. Even if we need minimal OwnerId in the future, equality contract will stay the same - only "owner id" determines the equality, not it's properties.

At the same time, if the goal of this PR is stabilization of thread_aware, and you already suggesting breaking changes, we should add Builder type right away, and may constructor methods private.

It's purely additive change, but I can see a value of adding Owner::builder() now with min_threads being set to 0 by default.

Moreover, unless there is a defendable use case for min_threads, it's better to remove it from the type completely. Adding new fields when we have builder is not a breaking change, while removing it is.

Ralf Biedert (@ralfbiedert) Our scenario for min_threads is allocating collections with initial capacity. For thread_aware::Arc we cannot pre-initialize the collection because at the point of Arc creation we don't know the coordinates. Current solution is to create underlying collection on demand:

slots: OnceLock<Box<[Slot<T>]>>,

Is that work the extra complexity? Could we just get away without exposing min_threads initially?

Copilot AI review requested due to automatic review settings August 31, 2026 13:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

crates/thread_aware_core/src/thread_aware.rs:165

  • The #[cfg(test)] mod tests block is missing the standard #[cfg_attr(coverage_nightly, coverage(off))] guard used elsewhere in the workspace to prevent test-only code from counting against the 100% coverage gate.
#[cfg(test)]
mod tests {

Comment on lines +52 to +63
/// Without `std` the thread id is gone, so equality and hashing cover the owner and the
/// NUMA node alone. `Thread` cannot be constructed here, which is exactly why that
/// narrowing is unobservable in a real program, so pin the shape by size instead: anything
/// wider than the two remaining fields means the id came back.
#[cfg(not(feature = "std"))]
#[test]
fn without_std_a_thread_carries_no_thread_id() {
let align = align_of::<Thread>();
let expected = (size_of::<Owner>() + size_of::<NumaNode>()).div_ceil(align) * align;

assert_eq!(size_of::<Thread>(), expected, "a no_std `Thread` is its owner and its NUMA node");
}
//!
//! # Library authors: implementing the trait
//!
//! **Library and application authors** implement [`ThreadAware`], usually through the

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖: Non-blocking — The documented derive still implements the incompatible thread_aware::ThreadAware, not this crate's borrowed-Thread trait.

Please mark derive support as pending until thread_aware adopts this crate, and regenerate the README.

impl_thread_aware!(NumaNode);
#[cfg(any(test, feature = "std"))]
impl_thread_aware!(std::thread::ThreadId);
impl_thread_aware!(Thread);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖: Non-blocking — No-op implementations for Thread, Owner, NumaNode, and ThreadId let derived cached-coordinate fields retain their old location after relocation.

Please document that cached-coordinate fields require hand-written relocate rather than field forwarding.

impl_thread_aware!(NumaNode);
#[cfg(any(test, feature = "std"))]
impl_thread_aware!(std::thread::ThreadId);
impl_thread_aware!(Thread);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖: Non-blocking — Omitting PhantomData blocks derived implementations for generic marker fields; orphan rules prevent the higher-level crate from adding the implementation later.

Please add the incumbent no-op implementation and boundary tests, or document the exclusion.

@martin-kolinek martin-kolinek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖: Approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agency-rocket Touched by a rocket skill

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants